Skip to content

Upgrading minimatch version to fix MSRC vulnerability issue #21035

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 13 commits into from
May 29, 2025
Merged

Upgrading minimatch version to fix MSRC vulnerability issue #21035

merged 13 commits into from
May 29, 2025

Conversation

rishabhmalikMS
Copy link
Contributor

@rishabhmalikMS rishabhmalikMS commented May 20, 2025 •
edited
Loading

Context

Upgrading minimatch version to fix MSRC vulnerability issue.
📌 How to link to ADO Work Items
AB#2281543

Task Name

ExtractFilesV1, GulpV0, GulpV1, XamarinTestCloudV1

Description

A ReDoS vulnerability exists in the braceExpand function of the minimatch package, which can be exploited using crafted input patterns.
Fix: Upgrade minimatch to version 3.0.5 or later.
ICM link for reference: https://portal.microsofticm.com/imp/v5/incidents/details/31000000365389/summary

Key minimatch v4 changes:

  • ** (globstar) now matches zero or more path segments (previously one or more). We already have pipeline tests using ** patterns, and these are passing, confirming our patterns work as expected with the new globstar behavior.
  • New option: allowWindowsEscape (not used in our code).
  • Fix bug with escaped '@' in patterns (Not related to our implementation)
  • nocase: true is always treated as "magic" (we only use nocase on Windows in our code, which is expected).
  • Internal marker exposure and improved pattern parsing (not relevant to our usage).

Canary runs
ExtractFiles: https://dev.azure.com/canarytest/PipelineTasks/_build/results?buildId=162239&view=results
GulpV0: https://dev.azure.com/canarytest/PipelineTasks/_build/results?buildId=162240&view=results
GulpV1: https://dev.azure.com/canarytest/PipelineTasks/_build/results?buildId=162241&view=results

Risk Assessment (Low / Medium / High)

Medium. Major version upgrade of minimatch package. Tested by running canary tests pipeline and unit tests for updated tests.

Unit Tests Added or Updated (Yes / No)

No new tests added

Additional Testing Performed

Tested by running canary tests pipeline and unit tests for updated tests

Documentation Changes Required (Yes / No)

No

Checklist

  • Related issue linked (if applicable)
  • Task version was bumped — see versioning guide
  • Verified the task behaves as expected

@rishabhmalikMS rishabhmalikMS requested review from tarunramsinghani and a team as code owners May 20, 2025 07:00
@rishabhmalikMS
Copy link
Contributor Author

/azp run

@rishabhmalikMS rishabhmalikMS changed the title Users/rishabhmalik/pkg version upgrade Upgrading minimatch version to fix MSRC vulnerability issue May 22, 2025
@rishabhmalikMS rishabhmalikMS enabled auto-merge (squash) May 23, 2025 05:51
@MantavyaDh
Copy link
Contributor

Is there any specific reason we are updating to version 3.0.5, the latest version is 10.0.1
Can we have a look at this once. change log

@tarunramsinghani
Copy link
Contributor

Please do share the canary pipeline links for the tasks changed.

@rishabhmalikMS
Copy link
Contributor Author

/azp run

@rishabhmalikMS
Copy link
Contributor Author

Is there any specific reason we are updating to version 3.0.5, the latest version is 10.0.1 Can we have a look at this once. change log

Latest versions are causing issues in pattern matching. Hence, we have use v4.

MantavyaDh
MantavyaDh reviewed May 26, 2025
View reviewed changes
@rishabhmalikMS
Copy link
Contributor Author

/azp run

@tarunramsinghani
Copy link
Contributor

Since we hae updated Major version the Risk should not be Low, and it should atleast be medium, BTW did you check what other changes are there in V4 and validated the task code that the changes does not impact the functionality

@rishabhmalikMS
Copy link
Contributor Author

Since we hae updated Major version the Risk should not be Low, and it should atleast be medium, BTW did you check what other changes are there in V4 and validated the task code that the changes does not impact the functionality

Key minimatch v4 changes:

  • ** (globstar) now matches zero or more path segments (previously one or more). We already have pipeline tests using ** patterns, and these are passing, confirming our patterns work as expected with the new globstar behavior.
  • New option: allowWindowsEscape (not used in our code).
  • Fix bug with escaped '@' in patterns (Not related to our implementation)
  • nocase: true is always treated as "magic" (we only use nocase on Windows in our code, which is expected).
  • Internal marker exposure and improved pattern parsing (not relevant to our usage).

@sanjuyadav24
Copy link
Contributor

Hi @rishabhmalikMS
Few of the images skipped in test pipeline, could you please check and re-run the pipeline with all images

@MantavyaDh
Copy link
Contributor

MantavyaDh commented May 28, 2025 •
edited
Loading

Hi @rishabhmalikMS Few of the images skipped in test pipeline, could you please check and re-run the pipeline with all images

I investigated this, it is due to the variables missing those pools which are set in the pipelines, canary pipelines
And these have been skipped in past as well for these tasks. We need to update the variables in the master branch for the tasks that are skipping.

@sanjuyadav24
Copy link
Contributor

Hi @rishabhmalikMS Few of the images skipped in test pipeline, could you please check and re-run the pipeline with all images

I investigated this, it is due to the variables missing those pools which are set in the pipelines, canary pipelines And these have been skipped in past as well for these tasks. We need to update the variables in the master branch for the tasks that are skipping.

okay, lets create a task to update these variables

@rishabhmalikMS rishabhmalikMS merged commit 5c77ece into master May 29, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MantavyaDh MantavyaDh MantavyaDh left review comments

@sanjuyadav24 sanjuyadav24 sanjuyadav24 approved these changes

@tarunramsinghani tarunramsinghani Awaiting requested review from tarunramsinghani tarunramsinghani is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rishabhmalikMS @MantavyaDh @tarunramsinghani @sanjuyadav24